tools: add macosx-firewall script to avoid popups #10114

danbev · 2016-12-04T18:04:31Z

Checklist

make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

tools

Description of change

Currently, there are a number of popups that get displayed when running
the tests asking to accept incoming network connections. Rules can be
added manually to the socket firewall on Mac OS X but getting this right
might not be obvious and quite a lot of time can be wasted trying to get
the rules right. This script hopes to simplify things a little so that
it can be re-run when needed.

The script should be runnable from both the projects root directory and
from the tools directory, for example:
$ sudo ./tools/macosx-firewall.sh

Fixes: #8911

Currently, there are a number of popups that get displayed when running the tests asking to accept incoming network connections. Rules can be added manually to the socket firewall on Mac OS X but getting this right might not be obvious and quite a lot of time can be wasted trying to get the rules right. This script hopes to simplify things a little so that it can be re-run when needed. The script should be runnable from both the projects root directory and from the tools directory, for example: $ sudo ./tools/macosx-firewall.sh Fixes: nodejs#8911

bnoordhuis · 2016-12-05T07:36:04Z

BUILDING.md

@@ -23,6 +23,7 @@ On OS X, you will also need:
  * You also need to install the `Command Line Tools` via Xcode. You can find
    this under the menu `Xcode -> Preferences -> Downloads`
  * This step will install `gcc` and the related toolchain containing `make`
+  * You may want to setup [firewall rules](tools/macosx-firewall.sh) to avoid popups asking to accept incoming network connections when running tests.


bnoordhuis · 2016-12-05T07:37:06Z

tools/macosx-firewall.sh

+TOOLSDIR="`( cd \"$TOOLSDIR\" && pwd) `"
+ROOTDIR="`( cd \"$TOOLSDIR/..\" && pwd) `"
+OUTDIR=$TOOLSDIR/../out
+OUTDIR="`( cd \"$OUTDIR\" && pwd) `"


Is the cd + pwd to ensure the directory exists?

It is really so that the path added is the full path without the .. in it. This is what is displayed when the command is run (if using .. in that path that is):

Application at path ( /Users/danielbevenius/work/nodejs/node/tools/../out/Debug/node ) added to firewall

But the entry when running /usr/libexec/ApplicationFirewall/socketfilterfw --listapps shows up without the path. Running the script again will not remove these paths, instead the new rules will just be added and the list will grow. But with the full path (without ..) it works as expected and the rules are removed and added properly.

bnoordhuis · 2016-12-05T07:39:51Z

tools/macosx-firewall.sh

+
+  $SFW --unblock $NODE_DEBUG
+  $SFW --unblock $NODE_RELEASE
+  $SFW --unblock $NODE_LINK


Can you wrap the env vars in quotes? This won't work if the paths have spaces in them.

Will do, thanks

bnoordhuis

LGTM with a request.

bnoordhuis · 2016-12-05T10:50:40Z

tools/macosx-firewall.sh

+TOOLSDIR="`( cd \"$TOOLSDIR\" && pwd) `"
+ROOTDIR="`( cd \"$TOOLSDIR/..\" && pwd) `"
+OUTDIR="$TOOLSDIR/../out"
+OUTDIR="`( cd \"$OUTDIR\" && pwd) `"


Can you add a comment explaining the use of pwd (what you wrote in your comment earlier)?

Fishrock123 · 2016-12-05T14:41:58Z

BUILDING.md

@@ -23,6 +23,8 @@ On OS X, you will also need:
  * You also need to install the `Command Line Tools` via Xcode. You can find
    this under the menu `Xcode -> Preferences -> Downloads`
  * This step will install `gcc` and the related toolchain containing `make`
+  * You may want to setup [firewall rules](tools/macosx-firewall.sh) to avoid
+    popups asking to accept incoming network connections when running tests.


Maybe this should specify a bit more how to run it an some implications?

@Fishrock123 I've added as suggestion, let me know what you think. Thanks

Also a short description of what the script does.

jasnell · 2016-12-05T23:30:25Z

BUILDING.md

+```console
+$ sudo ./tools/macosx-firewall.sh
+```
+Running this script will add rules for the executable `node` in the out directory and the symbolic `node` link in the projects root directory.


long line here... can you please wrap at 80 chars

Sorry about that, was using a MarkDown editor and did not notice.

I forgot about cctest which will also show up at times.

danbev · 2016-12-07T19:42:37Z

@Fishrock123 Do the changes look aright to you? If so would you mind approving or giving a LGTM and I'll merge this. Thanks

Currently, there are a number of popups that get displayed when running the tests asking to accept incoming network connections. Rules can be added manually to the socket firewall on Mac OS X but getting this right might not be obvious and quite a lot of time can be wasted trying to get the rules right. This script hopes to simplify things a little so that it can be re-run when needed. The script should be runnable from both the projects root directory and from the tools directory, for example: $ sudo ./tools/macosx-firewall.sh Fixes: nodejs#8911 PR-URL: nodejs#10114 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

danbev · 2016-12-09T07:35:35Z

Landed in a8137dd

Currently, there are a number of popups that get displayed when running the tests asking to accept incoming network connections. Rules can be added manually to the socket firewall on Mac OS X but getting this right might not be obvious and quite a lot of time can be wasted trying to get the rules right. This script hopes to simplify things a little so that it can be re-run when needed. The script should be runnable from both the projects root directory and from the tools directory, for example: $ sudo ./tools/macosx-firewall.sh Fixes: #8911 PR-URL: #10114 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

nodejs-github-bot added doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory. labels Dec 4, 2016

mscdex changed the title ~~tools: add macosx-firwall script to avoid popups~~ tools: add macosx-firewall script to avoid popups Dec 4, 2016

Fishrock123 added the macos Issues and PRs related to the macOS platform / OSX. label Dec 4, 2016

bnoordhuis requested changes Dec 5, 2016

View reviewed changes

danbev added 2 commits December 5, 2016 09:48

fix long line

c39bf95

quote variables to allow for paths with spaces

d255df9

bnoordhuis approved these changes Dec 5, 2016

View reviewed changes

Fishrock123 reviewed Dec 5, 2016

View reviewed changes

danbev added 2 commits December 5, 2016 20:34

add comment explaining the usage of cd and pwd

e546655

show usage os running the script

4f29ee5

Also a short description of what the script does.

jasnell reviewed Dec 5, 2016

View reviewed changes

fix lone line

46ad909

jasnell approved these changes Dec 6, 2016

View reviewed changes

add cctest to firewall script

cef43de

I forgot about cctest which will also show up at times.

gibfahn approved these changes Dec 9, 2016

View reviewed changes

cjihrig approved these changes Dec 9, 2016

View reviewed changes

danbev closed this Dec 9, 2016

italoacasas mentioned this pull request Dec 15, 2016

v7.3.0 proposal #10277

Merged

MylesBorins added land-on-v4.x labels Jan 16, 2017

danbev deleted the mac-os-x-firewall-script branch January 17, 2017 07:18

This was referenced Jan 24, 2017

v4.8.0 proposal #10973

Merged

v6.10.0 proposal #10974

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tools: add macosx-firewall script to avoid popups #10114

tools: add macosx-firewall script to avoid popups #10114

danbev commented Dec 4, 2016

bnoordhuis Dec 5, 2016

bnoordhuis Dec 5, 2016

danbev Dec 5, 2016

bnoordhuis Dec 5, 2016

danbev Dec 5, 2016

bnoordhuis left a comment

bnoordhuis Dec 5, 2016

Fishrock123 Dec 5, 2016

danbev Dec 5, 2016

jasnell Dec 5, 2016

danbev Dec 6, 2016

danbev commented Dec 7, 2016

danbev commented Dec 9, 2016

tools: add macosx-firewall script to avoid popups #10114

tools: add macosx-firewall script to avoid popups #10114

Conversation

danbev commented Dec 4, 2016

Checklist

Affected core subsystem(s)

Description of change

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnoordhuis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danbev commented Dec 7, 2016

danbev commented Dec 9, 2016